Skip to content

Comments

Fix some JET errors around matching methods for send_connection_hdr(...) and send_msg_now(...) (underlying problem: the Worker(...) constructors might not always return a Worker)#180

Merged
DilumAluthge merged 2 commits intomasterfrom
dpa/fix-jet-send_connection_hdr
Feb 21, 2026
Merged

Fix some JET errors around matching methods for send_connection_hdr(...) and send_msg_now(...) (underlying problem: the Worker(...) constructors might not always return a Worker)#180
DilumAluthge merged 2 commits intomasterfrom
dpa/fix-jet-send_connection_hdr

Conversation

@DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Feb 20, 2026

Cherry-picked out of #164

┌ create_worker(manager::Distributed.ClusterManager, wconfig::Distributed.WorkerConfig) @ Distributed ~/Distributed.jl/src/cluster.jl:709
│ no matching method found `send_connection_hdr(::Distributed.LocalProcess, ::Bool)` (1/2 union split): Distributed.send_connection_hdr(w::Union{Distributed.LocalProcess, Distributed.Worker}, true)
└────────────────────
┌ create_worker(manager::Distributed.ClusterManager, wconfig::Distributed.WorkerConfig) @ Distributed ~/Distributed.jl/src/cluster.jl:712
│ no matching method found `send_msg_now(::Distributed.LocalProcess, ::Distributed.MsgHeader, ::Distributed.JoinPGRPMsg)` (1/2 union split): Distributed.send_msg_now(w::Union{Distributed.LocalProcess, Distributed.Worker}, Distributed.MsgHeader(Distributed.RRID(0, 0)::Distributed.RRID, ntfy_oid::Distributed.RRID)::Distributed.MsgHeader, join_message)
└────────────────────

The underlying problem seems to be that the Worker(...) constructors might not always return a Worker. See also: #182

@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.47%. Comparing base (231da28) to head (5cf8ca4).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #180   +/-   ##
=======================================
  Coverage   79.47%   79.47%           
=======================================
  Files          10       10           
  Lines        1959     1959           
=======================================
  Hits         1557     1557           
  Misses        402      402           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

end

w = Worker(w_stub.id, r_s, w_s, manager; config=wconfig)
w = Worker(w_stub.id, r_s, w_s, manager; config=wconfig)::Worker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this also be fixed by adding ::Worker return annotations to the constructors?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. I think we need to look at this code closer.

The problematic line seems to be line 143 here:

function Worker(id::Int, conn_func)
@assert id > 0
if haskey(map_pid_wrkr, id)
return map_pid_wrkr[id]
end
w=new(id, Threads.ReentrantLock(), [], [], false, W_CREATED, Threads.Condition(), time(), conn_func)
w.initialized = Event()
register_worker(w)
w
end
Worker() = Worker(get_next_pid())
end

Line 143 is this line:

return map_pid_wrkr[id] 

map_pid_wrkr is a global constant. The problem (I think) is the type of map_pid_wrkr:

const map_pid_wrkr = Dict{Int, Union{Worker, LocalProcess}}()

So when we index into map_pid_wrkr with map_pid_wrkr[id], how do we know whether map_pid_wrkr[id] is a Worker or a LocalProcess?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I absolutely hate that 🤮 But something to fix another time... We could safely add an annotation to this constructor though, which I think is the one getting called here:

function Worker(id::Int, r_stream::IO, w_stream::IO, manager::ClusterManager;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried adding ::Worker to that constructor (the constructor starting on line 124), but that's not sufficient. Here's the source code for that constructor:

function Worker(id::Int, r_stream::IO, w_stream::IO, manager::ClusterManager;
version::Union{VersionNumber, Nothing}=nothing,
config::WorkerConfig=WorkerConfig())
w = Worker(id)
w.r_stream = r_stream
w.w_stream = buffer_writes(w_stream)
w.w_serializer = ClusterSerializer(w.w_stream)
w.manager = manager
w.config = config
w.version = version
set_worker_state(w, W_CONNECTED)
register_worker_streams(w)
w
end

The problem is, on line 127, that constructor calls the Worker(::Int) constructor, which in turn calls the Worker(::Int, ::Any) constructor, and now we're in the Worker(::Int, ::Any) constructor, which is the constructor that includes the return map_pid_wrkr[id] line.

So, if I try adding ::Worker to the line 124 constructor, JET still complains, because Worker(::Int, ::Any) might return a LocalProcess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bleh, devastating.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened an issue: #182

…...)` and `send_msg_now(...)`

```
┌ create_worker(manager::Distributed.ClusterManager, wconfig::Distributed.WorkerConfig) @ Distributed ~/Distributed.jl/src/cluster.jl:709
│ no matching method found `send_connection_hdr(::Distributed.LocalProcess, ::Bool)` (1/2 union split): Distributed.send_connection_hdr(w::Union{Distributed.LocalProcess, Distributed.Worker}, true)
└────────────────────
```

```
┌ create_worker(manager::Distributed.ClusterManager, wconfig::Distributed.WorkerConfig) @ Distributed ~/Distributed.jl/src/cluster.jl:712
│ no matching method found `send_msg_now(::Distributed.LocalProcess, ::Distributed.MsgHeader, ::Distributed.JoinPGRPMsg)` (1/2 union split): Distributed.send_msg_now(w::Union{Distributed.LocalProcess, Distributed.Worker}, Distributed.MsgHeader(Distributed.RRID(0, 0)::Distributed.RRID, ntfy_oid::Distributed.RRID)::Distributed.MsgHeader, join_message)
└────────────────────
```

(cherry picked from commit 3e74cbc)
@DilumAluthge DilumAluthge force-pushed the dpa/fix-jet-send_connection_hdr branch from 1eaff36 to a18f0b9 Compare February 21, 2026 08:03
Copy link
Member

@JamesWrigley JamesWrigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! (with sadness)

@DilumAluthge DilumAluthge marked this pull request as ready for review February 21, 2026 08:41
@DilumAluthge DilumAluthge changed the title Fix some JET errors around matching methods for send_connection_hdr(...) and send_msg_now(...) Fix some JET errors around matching methods for send_connection_hdr(...) and send_msg_now(...) (underlying problem: the Worker(...) constructors might not always return a Worker) Feb 21, 2026
@DilumAluthge DilumAluthge merged commit d396a8c into master Feb 21, 2026
8 checks passed
@DilumAluthge DilumAluthge deleted the dpa/fix-jet-send_connection_hdr branch February 21, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants